Skip to content

Add generic SSL/TLS configuration support#17854

Draft
HTHou wants to merge 6 commits into
masterfrom
codex/generic-ssl-config
Draft

Add generic SSL/TLS configuration support#17854
HTHou wants to merge 6 commits into
masterfrom
codex/generic-ssl-config

Conversation

@HTHou

@HTHou HTHou commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR adds generic SSL/TLS configuration support across IoTDB client and service paths without introducing provider-specific behavior.

Changes include:

  • Add a shared RpcSslUtils helper for SSL context setup, optional JSSE provider registration, and keystore/truststore type detection.
  • Wire generic SSL/TLS protocol and provider options through Session, JDBC, CLI import/export tools, Thrift transport creation, REST HTTPS, and Ratis certificate loading.
  • Add generic ssl_protocol and ssl_provider_class configuration entries. Cipher suites are not exposed as a user-facing configuration; transports use the defaults selected by the configured SSL/TLS protocol and provider.
  • Add JDBC URL parsing coverage for the new SSL/TLS parameters.

Validation

  • ./mvnw spotless:apply -pl iotdb-client/service-rpc,iotdb-core/node-commons
  • ./mvnw -pl iotdb-client/service-rpc,iotdb-client/isession,iotdb-client/session,iotdb-client/jdbc,iotdb-client/cli,iotdb-core/node-commons -DskipTests compile
  • ./mvnw -pl iotdb-client/jdbc -Dtest=UtilsTest#testParseSslConfig test
  • git diff --check

@codecov

codecov Bot commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.34632% with 184 lines in your changes missing coverage. Please review.
✅ Project coverage is 40.83%. Comparing base (d563cd0) to head (4c3fe70).
⚠️ Report is 24 commits behind head on master.

Files with missing lines Patch % Lines
...rc/main/java/org/apache/iotdb/rpc/RpcSslUtils.java 13.04% 80 Missing ⚠️
...a/org/apache/iotdb/tool/data/AbstractDataTool.java 0.00% 17 Missing ⚠️
...g/apache/iotdb/tool/schema/AbstractSchemaTool.java 0.00% 13 Missing ⚠️
...java/org/apache/iotdb/tool/common/OptionsUtil.java 0.00% 9 Missing ⚠️
.../org/apache/iotdb/rpc/BaseRpcTransportFactory.java 0.00% 7 Missing ⚠️
...b/commons/service/AbstractThriftServiceThread.java 0.00% 5 Missing ⚠️
...va/org/apache/iotdb/session/SessionConnection.java 50.00% 4 Missing ⚠️
...ava/org/apache/iotdb/session/pool/SessionPool.java 42.85% 4 Missing ⚠️
...che/iotdb/db/conf/rest/IoTDBRestServiceConfig.java 0.00% 4 Missing ⚠️
...nt/cli/src/main/java/org/apache/iotdb/cli/Cli.java 0.00% 3 Missing ⚠️
... and 22 more
Additional details and impacted files 
@@             Coverage Diff             @@
##             master   #17854     +/-   ##
===========================================
  Coverage     40.83%   40.83%             
+ Complexity     2613      318   -2295     
===========================================
  Files          5201     5246     +45     
  Lines        353904   363358   +9454     
  Branches      45284    46798   +1514     
===========================================
+ Hits         144505   148369   +3864     
- Misses       209399   214989   +5590

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Caideyipi

Caideyipi commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

I think there are two correctness issues in this PR:

  1. ssl_protocol=TLS is used as both the SSLContext algorithm and Jetty enabled protocol for REST HTTPS. In RestService.configureSSL, the value is passed to sslContextFactory.setIncludeProtocols(protocol). The default template value is TLS, but JSSE enabled protocol names are normally TLSv1.3, TLSv1.2, etc., not TLS. So enabling REST HTTPS with the default config can leave Jetty with no matching enabled protocol / fail TLS handshakes. The context algorithm and enabled protocol list should be separate, or REST should not call setIncludeProtocols for the generic TLS value.

  2. The legacy SSL overloads in BaseRpcTransportFactory now explicitly call the new overload with null, null, so internal synchronous SSL clients still do not use ssl_protocol / ssl_provider_class. For example SyncDataNodeInternalServiceClient, SyncConfigNodeIServiceClient, and IoT consensus sync clients call the old overload while enable_internal_ssl is true. If a custom JSSE provider is required, the server side may register it through CommonDescriptor.configureRpcSsl(), but these clients create their SSL transport without that provider and can fail to connect. These internal clients should pass commonConfig.getSslProtocol() and commonConfig.getSslProviderClass() to the new overload.

HTHou
HTHou commented Jun 11, 2026
View reviewed changes
Comment thread iotdb-client/cli/src/main/java/org/apache/iotdb/cli/AbstractCli.java Outdated
Comment on lines +163 to +164
keywordSet.add("-" + SSL_PROTOCOL_ARGS);
keywordSet.add("--" + SSL_PROTOCOL_ARGS);

@HTHou HTHou Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why using “--”

@sonarqubecloud

sonarqubecloud Bot commented Jun 11, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@HTHou @Caideyipi